Use PublicationDistribution from pulpcore.plugin #219
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 9, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. Each receives a viewset, serializer, and filterset to match. Each also is a full Detail model and can be used by plugins without additional code from them. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 9, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
bmbouter
changed the title
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 9, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. The two models are not detail objects, and require the plugin writer to declare a Distribution detail object. It was already this way before this PR. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
bmbouter
pushed a commit
to bmbouter/pulpcore-plugin
that referenced
this pull request
May 9, 2019
Model Changes The Distribution model was replaced with the RepositoryVersionDistribution and PublicationDistribution models. These are both abstract models and are meant to be subclassed further. Serializer Changes The DistributionSerializer is removed and now BaseDistributionSerializer, PublicationDistributionSerializer, and RepositoryVersionDistributionSerializer are here. These can also be further subclassed by plugin writers. ViewSet Changes DistributionViewset is now called BaseDistributionViewSet. No other viewsets are provided. Required PR: pulp/pulpcore#124 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes #4785
bmbouter
pushed a commit
to bmbouter/pulpcore-plugin
that referenced
this pull request
May 9, 2019
Model Changes The Distribution model was replaced with the RepositoryVersionDistribution and PublicationDistribution models. These are both abstract models and are meant to be subclassed further. Serializer Changes The DistributionSerializer is removed and now BaseDistributionSerializer, PublicationDistributionSerializer, and RepositoryVersionDistributionSerializer are here. These can also be further subclassed by plugin writers. ViewSet Changes DistributionViewset is now called BaseDistributionViewSet. No other viewsets are provided. Docs Changes The release notes are updated to indicate the breaking changes. Required PR: pulp/pulpcore#124 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes #4785
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 9, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. The two models are not detail objects, and require the plugin writer to declare a Distribution detail object. It was already this way before this PR. This also adds a release note about the breaking changes that come with the switch to Master/Detail. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 9, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. The two models are not detail objects, and require the plugin writer to declare a Distribution detail object. It was already this way before this PR. This also adds a release note about the breaking changes that come with the switch to Master/Detail. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 9, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. The two models are not detail objects, and require the plugin writer to declare a Distribution detail object. It was already this way before this PR. This also adds a release note about the breaking changes that come with the switch to Master/Detail. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 9, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. The two models are not detail objects, and require the plugin writer to declare a Distribution detail object. It was already this way before this PR. This also adds a release note about the breaking changes that come with the switch to Master/Detail. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 9, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. The two models are not detail objects, and require the plugin writer to declare a Distribution detail object. It was already this way before this PR. This also adds a release note about the breaking changes that come with the switch to Master/Detail. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
dkliban
approved these changes
May 9, 2019
bmbouter
pushed a commit
to bmbouter/pulpcore-plugin
that referenced
this pull request
May 9, 2019
Model Changes The Distribution model was replaced with the RepositoryVersionDistribution and PublicationDistribution models. These are both abstract models and are meant to be subclassed further. Serializer Changes The DistributionSerializer is removed and now BaseDistributionSerializer, PublicationDistributionSerializer, and RepositoryVersionDistributionSerializer are here. These can also be further subclassed by plugin writers. ViewSet Changes DistributionViewset is now called BaseDistributionViewSet. No other viewsets are provided. Docs Changes The release notes are updated to indicate the breaking changes. Required PR: pulp/pulpcore#124 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes #4785
gmbnomis
reviewed
May 10, 2019
pulp_file/app/serializers.py
Outdated
| @@ -81,22 +81,31 @@ class FilePublicationSerializer(PublicationSerializer): | |||
| Serializer for File Publications. | |||
| """ | |||
|
|
|||
| _file_distributions = DetailRelatedField( | |||
There was a problem hiding this comment.
What do you think about just using the default relation name (i.e. no need for default_related_name) and make the field available at _distributions in the serializer:
_distributions = RelatedField(
help_text=_(
"This publication is currently being served as defined by these distributions."
),
source="filedistribution_set",
many=True,
read_only=True,
view_name="distributions-file/file-detail",
)As an alternative we could use a DetailRelatedField. It is simpler to use, although not really necessary in this case, since we don't need to support subclasses of FileDistribution.
There was a problem hiding this comment.
Since this is a plugin defined field, should we drop the underscore?
There was a problem hiding this comment.
Good question. Does a convention that should be implemented the same in all plugins make the field a "core field"? (we had a slightly similar discussion on _artifact once, IIRC). I am fine either way and slightly preferring to drop the underscore.
There was a problem hiding this comment.
From the feedback I think the best thing to do is to switch this to be distributions. That both makes it a core variable and also brings more consistency with the hope that more plugins will do the same. Does this sound right?
There was a problem hiding this comment.
yes, PR looks good to me now
The plugin API now uses PublicationDistribution which causes pulp_file users to only receive the fields they can actually use. Required PR: pulp/pulpcore#124 Required PR: pulp/pulpcore-plugin#97 https://pulp.plan.io/issues/4785 closes #4785
bmbouter
pushed a commit
to bmbouter/pulpcore
that referenced
this pull request
May 10, 2019
The Distribution contained options which should not be mixed. Specifically the repository and repository_version options go together and the publication goes by itself. This PR splits that object into two new objects RepositoryVersionDistribution and PublicationDistribution. The two models are not detail objects, and require the plugin writer to declare a Distribution detail object. It was already this way before this PR. This also adds a release note about the breaking changes that come with the switch to Master/Detail. Required PR: pulp/pulpcore-plugin#97 Required PR: pulp/pulp_file#219 https://pulp.plan.io/issues/4785 closes pulp#4785
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The plugin API now uses PublicationDistribution which causes pulp_file
users to only receive the fields they can actually use.
Required PR: pulp/pulpcore#124
Required PR: pulp/pulpcore-plugin#97
https://pulp.plan.io/issues/4785
closes #4785